feat(media-use): usage visibility — shared telemetry identity, miss log, resolve --stats#2113
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at 5e42659 (R1).
Third PR in the media-use stack (base = #2065). Wires up usage visibility:
- Migrates the media-use anon-id from
~/.media/anon-id→ the CLI/studio-shared~/.hyperframes/config.json.anonymousId, so a person is one PostHog profile across surfaces (verified: CLI atpackages/cli/src/telemetry/config.ts:17,70,87,98uses the same field name at the same path). - Adds
$identifylinking anon → HeyGen account (email or username) on sign-in — read from~/.heygen/credentials.userper the CLI's own store schema (packages/cli/src/auth/store.ts:75hasKNOWN_USER_KEYS = {email, first_name, last_name, username}), so the schema assumption is correct. - New
resolve --stats(human +--json) +--days Nwindow: local report from.media/+~/.media+ a new~/.media/misses.jsonllocal miss log. Miss log captures intent text (local only) while the telemetry eventmedia_use_resolve_missdeliberately does NOT — the dashboard doc calls this pairing out explicitly. Clean separation. - New
~/.media/telemetry-notice-shownmarker for the media-use first-run stderr notice. - New
references/telemetry-dashboard.md— reproducible PostHog dashboard definition. Good doc.
Rubric verifications:
- Cohort mixing / shared identity — intentional; every event carries
surface: "media-use"for facetability. ✅ - Sibling-metric tag-set symmetry —
resolvecarriestype/source/provider/via/local_only/provider_override;resolve_misscorrectly dropsproviderandvia(there is none for a miss); doctor carriesok/checks_failed/failed[]. Consistent. ✅ - Field conflation / verify at writer —
anonymousId()reads from and writes to the exact shared config field the CLI reads/writes. ✅ - Hardcoded default in new writer — none;
$identifyonly fires when a real email/username is present. ✅
Four inline findings (2 🟠 + 2 🟡):
- 🟠 Notice-shown state is split from the shared config (
~/.media/telemetry-notice-shownvs CLI'sconfig.telemetryNoticeShown). Users who saw the CLI notice will see media-use's separately — undermines the "one profile" architecture. - 🟠 No migration path from the old
~/.media/anon-idfile. media-use-only users get a fresh persona ID on upgrade; PostHog will show a cohort discontinuity on merge day. - 🟡
--daysdoesn't validate — negative / zero / non-numeric silently produces empty stats that look like "no activity." - 🟡
buildStatsouter catch zeroes the whole report on any error, no signal that partial data failed to load.
Meta: PR body is a template stub ("Brief description of the change"). Reviewers have to read the diff top-to-bottom to figure out scope. Fill it in before merge — the shape is unusually large for the effort of a body.
CI: preview-regression + regression pass; regression-shards + Preflight lint-format skipping via detect-changes gate; Graphite mergeability check pending only. No red required checks. mergeable = MERGEABLE.
Zero blockers. Ready from where I sit once the 🟠s are decided and the body is filled in.
| } | ||
| } | ||
|
|
||
| function showTelemetryNotice() { |
There was a problem hiding this comment.
🟠 The first-run notice uses a media-use-specific marker file at ~/.media/telemetry-notice-shown, but the shared CLI config at ~/.hyperframes/config.json already has a telemetryNoticeShown: boolean field that packages/cli/src/telemetry/client.ts:185 reads to gate the CLI's own notice. Because this PR's whole thesis is "one PostHog profile across surfaces via the shared config," splitting the notice-shown state undermines it: a user who's already seen the CLI's notice (and whose config shows telemetryNoticeShown: true) will still see the media-use notice on first resolve --stats run. Two clean options: (a) read/write config.telemetryNoticeShown here — one notice per identity across surfaces; (b) if the wording needs to stay media-use-specific (mentions HeyGen account linking), add a mediaUseNoticeShown field to the same shared config instead of a separate file, so the state travels with the identity.
Nit-adjacent side effect: if the marker-write at line 92 ever fails (read-only home, exotic FS), the notice re-prints on every event. console.error on every event isn't great UX. Aligning with config.telemetryNoticeShown fixes both.
— Review by Rames D Jusso
|
|
||
| // Stable per-machine anonymous id, persisted in the dir media-use already owns. | ||
| // Stable per-machine anonymous id, read from the shared CLI/studio contract. | ||
| function anonymousId() { |
There was a problem hiding this comment.
🟠 No migration from the previous ~/.media/anon-id file. media-use-only users on the pre-#2113 version have a stable UUID there; after this PR ships, if ~/.hyperframes/config.json doesn't already exist (e.g. they've never run the CLI, only media-use), anonymousId() seeds a fresh UUID and their prior activity is orphaned in PostHog. The test at telemetry.test.mjs:98-107 covers the case where BOTH files exist (shared config wins — correct) but not the case where ONLY the old marker exists. Consider: if (!parsed.anonymousId && existsSync(join(homedir(), ".media/anon-id"))) → adopt the old UUID into the shared config, preserving cohort continuity. Otherwise the dashboards will show a step-function of "new users" on merge day that isn't a real signup spike.
— Review by Rames D Jusso
| if (args.stats) { | ||
| const report = buildStats({ | ||
| projectDir, | ||
| days: args.days ? Number(args.days) : undefined, |
There was a problem hiding this comment.
🟡 --days N accepts anything the flag parser hands over — but the downstream math has silent failure modes: Number("garbage") === NaN → cutoff = null → window silently ignored (no error, user thinks their filter worked); Number("-5") = -5 → cutoff = future ISO → all past records fail time >= cutoff → stats show near-zero (looks like "no activity"); Number("0") → same result. All three read as "my project has almost no activity" instead of "the flag was invalid." Consider validating at flag-parse time: if (args.days !== undefined && !(Number(args.days) > 0)) { console.error("--days must be a positive integer"); process.exit(2); }. Non-blocking; --stats is a diagnostic tool so the affected user quickly notices "why is my hit rate 0%," but the fail-loud shape saves the debug session.
— Review by Rames D Jusso
|
|
||
| return report; | ||
| } catch { | ||
| return emptyReport(); |
There was a problem hiding this comment.
🟡 The outer catch at buildStats returns emptyReport() on any thrown error inside the try. If — say — ~/.media/manifest.jsonl has a subtle parse issue that readManifest throws on, or the global-cache read blows up, the entire report is zeroed and printed as total resolves: 0 / misses: 0 / hit rate: n/a. That reads to a user as "no activity," not "failed to load some of your data." Consider narrower per-step try/catch (project manifest / misses / global manifest each in their own guard, each falling back to an empty component) so partial data still surfaces and the user notices something's off. Or at minimum, distinguish empty-report-because-empty from empty-report-because-error with a _partial: true marker + a stderr log.
— Review by Rames D Jusso
vanceingalls
left a comment
There was a problem hiding this comment.
R1 — hyperframes #2113 at 5e426596923fe48dae19f1c3bf7c75bbd4dc102f
Landing after Rames D Jusso's R1 at 21:34Z. Same head SHA, concurring on his four inline findings (2 🟠 + 2 🟡) with independent verification. My lens on this one is identity + disclosure + PII-boundary mechanic; Rames already covered the cohort-continuity, sibling-metric symmetry, and stats robustness angles. Not repeating his rubric — extending where I have independent signal.
Concurring with Rames — all four verified
-
🟠 Notice-shown state split (
telemetry.mjs:78) — verified and this is the tightest of the four. The whole thesis of the PR (per the comment at:1-7) is "the same install id from ~/.hyperframes/config.json, plus a $identify to the HeyGen account on sign-in, so a person is one PostHog profile across surfaces." ButshowTelemetryNoticewrites its shown-marker to a media-use-owned path (~/.media/telemetry-notice-shown) instead of a boolean on the shared config. A user who's already run the hyperframes CLI and dismissed its telemetry notice — same person, same identity, per this PR's own architecture — gets re-noticed on their firstmedia-useinvocation. Rames's option (a) (share theconfig.telemetryNoticeShownfield) is the identity-preserving move; option (b) (a distinctmediaUseNoticeShownfield on the same shared config) is defensible if the disclosure text needs to stay media-use-specific because it names the HeyGen $identify linkage. My preference: (b) — the media-use notice explicitly discloses the account-linking behavior that the CLI notice doesn't, so the disclosures aren't interchangeable. But keeping the shown-state in the shared config either way. Non-blocking, but architectural-consistency worth naming. -
🟠 No migration from
~/.media/anon-id— verified.anonymousId()attelemetry.mjs:37-58reads~/.hyperframes/config.json, but never consults the pre-existing~/.media/anon-idfile that the previous version oftelemetry.mjswrote to. Any media-use-only user (i.e. someone who has runresolvebut never the hyperframes CLI or studio) has a stable UUID in.media/anon-idthat will be orphaned on this upgrade. Test attelemetry.test.mjs:98-107covers "shared config wins when both files exist" but not "adopt legacy when only the legacy file exists" — that's the missing path. Rames's suggested branch (if (!parsed.anonymousId && existsSync(join(homedir(), ".media/anon-id")))→ read + copy into shared config) preserves cohort continuity. Concurring; the fix is one branch plus one test. From my identity-mechanic lens: this is essentially a schema migration masquerading as a config read. If it doesn't get done in this PR, whoever writes the PostHog dashboard next month is going to see a step-function of "new users" on merge day that isn't real activity. -
🟡
--daysvalidation (resolve.mjs:147) — verified.Number("garbage") === NaN,Number("-5") === -5,Number("0") === 0all pass throughresolve.mjs:145'sargs.days ? Number(args.days) : undefinedand then intostats.mjs:80-82, which doesNumber(now) - Number(days) * 24 * 60 * 60 * 1000. The three failure modes Rames names (NaN → null cutoff silently, negative → future cutoff → all records fail window, zero → same as negative) all produce a stats report that reads as "no activity." A--statsdiagnostic that renders zero on a bad--daysis worse than useful — it lies to the user in the shape of a valid answer. Rames's fail-loud fix at flag-parse time (if (args.days !== undefined && !(Number(args.days) > 0)) exit(2)) is the right shape. Concurring. -
🟡
buildStatsouter catch swallowing errors (stats.mjs:116) — verified.try { ... } catch { return emptyReport(); }at:76-120is the outermost guard. IfreadManifest(projectDir)throws on a subtle JSONL parse issue, orreadGlobalManifest()blows up mid-walk, ordiskBytesthrows unexpectedly, the ENTIRE report zeroes out and the user seestotal resolves: 0 / misses: 0 / hit rate: n/a. Reads as "no activity," not "some data failed to load." I'd initially clocked the outer catch as intentional-fail-safe; Rames is right that it swallows the wrong class of failure. Per-step try/catch with_partial: truemarker in the JSON output (and a(partial data — some sources failed to load)line in the human output) is the correct shape. Concurring.
Identity + disclosure mechanic — my lens
Where Rames covered the verifier-of-schema angle (correctly linking ~/.heygen/credentials.user to packages/cli/src/auth/store.ts:75's KNOWN_USER_KEYS), I want to layer the disclosure semantic verification alongside:
- The first-run notice (
telemetry.mjs:76-98) explicitly enumerates what's tracked (type/source/provider) AND what's not (intent text, file names, or paths) AND what changes on sign-in (links to your account email or username) AND how to opt out. That's the four disclosure elements a "coarse pseudonymous + account-linked-on-sign-in" model needs. Verified against the actual event shapes atresolve.mjs:344-355(miss path — no intent),:1035(hit path — no intent, no filename),:766(candidates — counts only). The disclosure text matches the code truth. Good. $identifyuses the raw email asdistinct_id(viaheygenAccountDistinctIdat:60-74). That's the standard PostHog identity-stitching move — but it does put the email in PostHog data-at-rest in cleartext. Disclosed to the user, so consented. If the team ever wants to soften data-at-rest posture without sacrificing profile stitching, hashing the email asdistinct_idwhile adding a person-propertyheygen_usernamegets the same stitching without cleartext. Purely a forward note; the current design is honest and non-surprising and does what the notice says it does.- Notice fires on first
track()call, so--statsand--doctorinteract with it differently.--doctorcallstrack("media_use_doctor_run", ...)atresolve.mjs:131→ notice fires.--statsdoes NOT calltrack()— so a user whose first invocation is--statsgets local aggregates without ever seeing the disclosure. Two ways to close this: (a) callshowTelemetryNotice()from the--statsbranch too — one line atresolve.mjs:144, and semantically fits since--statsis itself a usage-visibility affordance; (b) live with the gap since--statsdoesn't itself send anything to PostHog. Not blocking, but (a) is one line and free. $ip: nullis stamped per event attelemetry.mjs:141— IP is suppressed at ingest, not just filtered downstream. Verified. Good.- Concurrency on
~/.hyperframes/config.jsonwrite: two processes racing onwriteFileSynccan drop the loser's UUID. Since both read on next boot, this converges — but the window produces one $identify event that never subsequently reconciles. Very minor, real, not blocking. Same class of issue Rames's migration concern raises in a different flavor: any time the shared config is a writer surface for multiple tools, races are possible. If the shared-config schema grows, a file-lock on the hyperframes-CLI side would be a defense.
Miss log — local-only, PII-safe
Not repeating Rames's coverage of the "intent local, telemetry coarse" separation — he named it and it's clean. Adding one small privacy note: ~/.media/misses.jsonl stores raw intent strings on disk unencrypted. On a shared machine or a synced volume ($HOME under iCloud Documents or Dropbox), those intents replicate. Not new to this PR — the manifest also lives under .media — but a future disclosure pass could clarify "not sent to PostHog" vs "stored locally in .media." Skip unless chasing a customer-privacy hardening pass.
--stats disk-walk
Nit adjacent to Rames's stats robustness concern: stats.mjs:65-72's diskBytes does a synchronous statSync per global-cache entry every time --stats runs. For a heavy cache (thousands of assets) that's a real filesystem walk on the hot path of the flag. Not a blocker. Mention only in case someone runs --stats in a tight loop from a script.
PR body
Rames flagged the template-stub body — concurring. The disclosure surface here is substantial (shared identity migration + PII boundary + new stats affordance) and reviewers a week from now can't reverse-engineer intent from the diff alone. Filling in the body before merge — with the "one profile across surfaces" thesis + the disclosure summary + a note about the migration path (assuming it lands) — is worth 15 minutes to get right.
What I'm NOT re-scoring
Rames's rubric already covers: cohort-mixing intent, sibling-metric tag-set symmetry, verify-at-writer for anonymousId, hardcoded-default check for $identify, CI signal. Consulted, concurred, moved on.
R1 by Via — clean thesis; four legitimate follow-ups from Rames all confirmed; disclosure design is honest and non-surprising.
… migration, stats robustness - Notice-shown state now lives in the shared ~/.hyperframes/config.json (config.telemetryNoticeShown, the CLI's own field) instead of a media-use-only ~/.media marker — so shared-identity users see the first-run notice once per person, not once per tool. - Migrate a pre-existing ~/.media/anon-id into the shared config on upgrade, so media-use-only users keep their PostHog persona instead of resetting. - buildStats: --days only windows on a positive finite value (negative/NaN → all time, not an empty report); dropped the top-level catch that masked a real error as an all-zero "no usage" report (sub-reads are individually guarded). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5k87mPZ4d6yiFwcWSb8Vv
4c8010b to
d8a8724
Compare
… migration, stats robustness - Notice-shown state now lives in the shared ~/.hyperframes/config.json (config.telemetryNoticeShown, the CLI's own field) instead of a media-use-only ~/.media marker — so shared-identity users see the first-run notice once per person, not once per tool. - Migrate a pre-existing ~/.media/anon-id into the shared config on upgrade, so media-use-only users keep their PostHog persona instead of resetting. - buildStats: --days only windows on a positive finite value (negative/NaN → all time, not an empty report); dropped the top-level catch that masked a real error as an all-zero "no usage" report (sub-reads are individually guarded). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5k87mPZ4d6yiFwcWSb8Vv
bc5bd2b to
05516bf
Compare
… migration, stats robustness - Notice-shown state now lives in the shared ~/.hyperframes/config.json (config.telemetryNoticeShown, the CLI's own field) instead of a media-use-only ~/.media marker — so shared-identity users see the first-run notice once per person, not once per tool. - Migrate a pre-existing ~/.media/anon-id into the shared config on upgrade, so media-use-only users keep their PostHog persona instead of resetting. - buildStats: --days only windows on a positive finite value (negative/NaN → all time, not an empty report); dropped the top-level catch that masked a real error as an all-zero "no usage" report (sub-reads are individually guarded). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5k87mPZ4d6yiFwcWSb8Vv
05516bf to
404aeac
Compare
bb8aeaf to
cddf270
Compare
… migration, stats robustness - Notice-shown state now lives in the shared ~/.hyperframes/config.json (config.telemetryNoticeShown, the CLI's own field) instead of a media-use-only ~/.media marker — so shared-identity users see the first-run notice once per person, not once per tool. - Migrate a pre-existing ~/.media/anon-id into the shared config on upgrade, so media-use-only users keep their PostHog persona instead of resetting. - buildStats: --days only windows on a positive finite value (negative/NaN → all time, not an empty report); dropped the top-level catch that masked a real error as an all-zero "no usage" report (sub-reads are individually guarded). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5k87mPZ4d6yiFwcWSb8Vv
404aeac to
a42f8e2
Compare
cddf270 to
ad9f8b2
Compare
…og, resolve --stats - U6: join the CLI/studio telemetry identity — read the shared install id from ~/.hyperframes/config.json (seed if absent) instead of a media-use-only ~/.media/anon-id, and $identify to the HeyGen account (email/username) once per run on sign-in. One PostHog person across surfaces; pseudonymous before sign-in, account-linked after. Event properties stay coarse (no intent/paths). - U1: one-time first-run disclosure to stderr + Privacy section in SKILL.md; honors DO_NOT_TRACK / HYPERFRAMES_NO_TELEMETRY. - U2: persist resolve misses to ~/.media/misses.jsonl (local → intent kept; the media_use_resolve_miss telemetry event stays intent-free). - U3: `resolve --stats` (+ --days) — local usage report over .media/ + ~/.media (volume by type, source/provider/via split, hit-rate, top missed intents, global-cache size/reuse); human + --json. - U4: reproducible PostHog dashboard definition (references/telemetry-dashboard.md). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5k87mPZ4d6yiFwcWSb8Vv
… migration, stats robustness - Notice-shown state now lives in the shared ~/.hyperframes/config.json (config.telemetryNoticeShown, the CLI's own field) instead of a media-use-only ~/.media marker — so shared-identity users see the first-run notice once per person, not once per tool. - Migrate a pre-existing ~/.media/anon-id into the shared config on upgrade, so media-use-only users keep their PostHog persona instead of resetting. - buildStats: --days only windows on a positive finite value (negative/NaN → all time, not an empty report); dropped the top-level catch that masked a real error as an all-zero "no usage" report (sub-reads are individually guarded). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5k87mPZ4d6yiFwcWSb8Vv
a42f8e2 to
98c286d
Compare

Summary
Makes media-use usage visible so we can tune it — a local
resolve --statsreport, on-disk miss logging, a reproducible PostHog dashboard, a first-run privacy disclosure, and (the core change) joining the shared CLI/studio telemetry identity so a person is one profile across surfaces.What / why
~/.hyperframes/config.json(seeds if absent) instead of a media-use-only~/.media/anon-id, and$identifys to the HeyGen account (email/username) once per run on sign-in. Pseudonymous before sign-in, account-linked after; event properties stay coarse (media type/source/provider — never intent text or paths).DO_NOT_TRACK/HYPERFRAMES_NO_TELEMETRY.~/.media/misses.jsonl(intent kept locally; themedia_use_resolve_missevent stays intent-free).resolve --stats(+--days) — volume by type, source/provider/via split, hit-rate, top missed intents, global-cache size/reuse; human +--json.references/telemetry-dashboard.md, grounded in events already flowing in PostHog project 356858.Legal/privacy
Not fully anonymous by design (must dedupe). Identified first-party product analytics tied to the HeyGen account on sign-in, opt-out + disclosed — flagged for a compliance sign-off before any external push.
Tests
node --testmedia-use suite: 148 pass / 0 fail (shared-config id, seed-if-absent,$identifyonce, signed-out, notice stderr-only, misses, stats).--statssmoke-tested against real~/.media.Stack (merge bottom→top): #2027 → #2065 → #2113